cc @serhiy-storchaka @corona10 @picnixz @cmaloney

I consider that the PR is now ready for a review :-)

I consider that the PR is now ready for a review :-)

How would you suggest reviewing? (this is a big diff!) Should we verify that no test coverage was lost etc?

A

How would you suggest reviewing? (this is a big diff!) Should we verify that no test coverage was lost etc?

Do you like how tests are grouped? test_xxx filename and content?

My PR was the impetus for this change, so it seemed appropriate that I scroll through it, but it's a large diff and I'm not familiar with most of the tests, so my review shouldn't count for much.

I didn't see anything technically wrong with the implementation. The test groups in the files seem reasonable to me. Many of the files end with a class PosixTester that could be named better and/or could have its test functions moved into one of the other test classes in that file. I don't think this matters given the file-level grouping, but if you ask, I will provide suggestions.

I think running the test coverage utilities on all the supported platforms would be useful to check that no coverage was lost. Is there a CI action for that?

The blank line spacing in the created files is inconsistent. Some classes have blank lines between the header and the first method, while others don't. Most classes are separated from the next by two lines, but some have three. I don't consider this important, but I don't maintain the tests. :)

I think splitting tests into separate files for each function is too much. We have separate classes for grouping.

I tried to have around 1,000 lines per Python file:

$ wc -l Lib/test/test_os/*.py|sort -n
     6 Lib/test/test_os/__init__.py
    22 Lib/test/test_os/utils.py
   121 Lib/test/test_os/test_encoding.py
   121 Lib/test/test_os/test_fspath.py
   183 Lib/test/test_os/test_random.py
   187 Lib/test/test_os/test_terminal.py
   205 Lib/test/test_os/test_link.py
   227 Lib/test/test_os/test_scheduler.py
   228 Lib/test/test_os/test_dirfd.py
   242 Lib/test/test_os/test_sendfile.py
   260 Lib/test/test_os/test_user.py
   275 Lib/test/test_os/test_macos_weaklink.py
   301 Lib/test/test_os/test_stat.py
   337 Lib/test/test_os/test_fd.py
   345 Lib/test/test_os/test_timerfd.py
   357 Lib/test/test_os/test_utime.py
   388 Lib/test/test_os/test_scandir.py
   401 Lib/test/test_os/test_environ.py
   497 Lib/test/test_os/test_misc.py
   518 Lib/test/test_os/test_file_attrs.py
   690 Lib/test/test_os/test_windows.py
   766 Lib/test/test_os/test_dirs.py
   954 Lib/test/test_os/test_file.py
  1033 Lib/test/test_os/test_process.py
  8664 total

In practice, files are between 100 and 500 lines except of test_windows, test_dirs, test_file and test_process (which are between 690 and 1,033 lines).

I see only one natural way to split the tests -- for functions implemented in C (in posix/nt modules) and pure Python functions defined in the os module.

I don't think that we can make test_os and test_posix under 1,000 lines by the way.

Maybe move some Windows and macOS specific tests into separate files -- if we are sure that they will never be relevant on other platforms.

test_windodws contain all tests which are specific to Windows.

@serhiy-storchaka is against this split, so I created #139453 which only creates test_windows.

Follow-up: I created #139480 to create test_os.test_process.